This gates on how many persons match, not on how big the table is. A team with 50M persons and a condition matching 600k samples about 9,375, falls under the threshold, and reruns exact over the whole table. That is the query this change exists to avoid. Has the exact rerun been timed on a team that size?

Measured now, from the ClickHouse query log rather than wall clock. Your scenario is live rather than hypothetical, and the answer splits in two.

The exact rerun you asked about is fine. On our largest internal project a channel-type person condition matches ~608k persons. Sampled 1-in-64 that is 9,554, under the 10,000 threshold, so it takes exactly the fallback path. That fallback runs 5.5-5.7 s and peaks at 3.1-3.6 GiB, nowhere near the per-query ceiling. Page cache is worth about 12%, so it is CPU-bound on decompression, not disk-bound. The id prefilter holds the dedup to the matched candidate set, which is why it survives.

The query that does fail is the other one. Every sizing request also runs the unfiltered denominator count over the whole person table, on both the empty-condition and the filtered code paths. On the largest projects that count fails with MEMORY_LIMIT_EXCEEDED at the per-query ceiling, in every observed run across a week. So a person condition on those projects has no working estimate today, and the fallback you flagged is not what is broken.

Your cost concern still stands, and the data sharpens it in a way I did not expect. person has no SAMPLE BY key, so the sampling predicate prunes no granules — every sampling rate reads the same rows and the same marks, confirmed by SelectedMarks. Sampling divides the aggregation state and nothing else. It cannot reduce scan cost, which means the probe you flagged in your other comment costs a full extra scan, and no change to the sampling rate would fix that. Computing both counts in one scan would; that is a follow-up, not this PR.

Worth knowing for the rollout: the same query log shows flags sizing queries on other projects failing on timeout and on the simultaneous-query limit rather than on memory. This PR does not help those, and the extra scan could hurt them.

One result came out clean. Three sampling rates against the same exact count of 608,628:

• 1-in-64 — 9,554 sampled, estimate 611,456, +0.46%
• 1-in-32 — 19,057 sampled, estimate 609,824, +0.20%
• 1-in-8 — 76,198 sampled, estimate 609,584, +0.16%

All inside the predicted standard error, and well inside the ~3% the comment claims.

Below the threshold this probe runs, gets discarded, and the exact query runs anyway. Any team under roughly 640k persons pays two counts where v1 paid one, and the editor calls this once per condition group. Worth keeping the flag scoped to teams that fail today.

Correct, and it is two counts per number rather than per request: a filtered condition sizes both the numerator and the denominator, so it is up to four queries where v1 ran two.

The query log makes this worse than it looks. person has no SAMPLE BY key, so the sampling predicate prunes no granules — the discarded probe reads the same rows and the same marks as the exact count that replaces it. It is a full extra scan, not a 64th of one, and no change to the sampling rate would fix that.

Not fixed in code here, because deciding to skip the probe needs the team size, which is what the probe is for. The fix that does work is computing both counts in one scan with a countDistinctIf, which drops the pair to one pass. That is a follow-up rather than this PR.

Taking your suggestion for the meantime: the rollout note on flags-blast-radius-query-v2 now records this as a rollout cost and says to step through projects large enough to fail today rather than a blanket ramp to 100.

TestSampledOrExactCount::test_a_small_sample_is_discarded_for_an_exact_count asserts moduli == [SAMPLE_MODULUS, None], so the discarded probe is visible in a test rather than only in the code.

With modulus 1 the multiply is a no op, so this still passes if the extrapolation is dropped entirely. A plain unit test driving the count callable with a stub would pin the multiplier and the threshold boundary.

Right, and the modulus-1 test was worth less than it looked. Replaced with TestSampledOrExactCount in test_person_sampling.py, which drives the callable with a stub and needs no database.

Checked by mutation rather than by eye:

• dropping * SAMPLE_MODULUS fails both extrapolation cases
• > in place of >= fails the "at the threshold" case alone

The ClickHouse test keeps its own job (that the gated path agrees with the v1 counts) and no longer carries the extrapolation claim.

Worth asserting this stays true. A regression here is silent, it just puts a flags fetch in front of an interactive request. The cache gates in this product already pin these two kwargs in their tests.

Added TestBlastRadiusGate, following test_both_gates_evaluate_locally_and_capture_nothing as you pointed at.

It pins a third thing alongside the two kwargs: groups and group_properties. The flag targets the project group, and local evaluation can only resolve that condition from the ids carried on the call, so dropping them makes the gate read False for every team whatever the rollout says — the same silent failure, one step further along.

All three checked by mutation: removing only_evaluate_locally, removing groups, or flipping send_feature_flag_events each fail it.

This also reaches the workflows batch person enumeration path, where a timed out page used to come back short and quietly end pagination. Right change, but it flips a workflows failure mode with no workflows test. It also surfaces as a 500 there, because that handler catches every exception.

Both parts are right, and the second one is why the change is worth making: has_more is len(page) == PERSON_BATCH_SIZE, so a short page reads as the end of the audience and the send skips every recipient after it. Silent truncation beats a loud failure only if nobody is counting on the recipients.

test_internal_user_blast_radius_persons_uses_workflows_query is now parameterized over the gate and asserts the settings the enumeration path actually receives, so gated on it must carry throw-on-timeout. Passing None there again fails the new case and leaves the gate-off case green. An actual ClickHouse timeout is not something I can force in a test, so this pins the wiring; TestPersonSamplingSettings pins the mode itself.

I left the 500 alone. That handler catching everything is pre-existing, and for this path a 500 is the outcome we want — the resolver sees the page failed instead of treating a short page as the end. Happy to narrow it if you would rather it were its own error class, but that felt like a separate change to the workflows error taxonomy.

This says HogQL cannot spill to disk, but the settings these counts use set a spill threshold and describe it as degrading to disk. Both cannot be true. The claim holds only for the default settings, which this path overrides.

You are right, and the defaults are the resolution: HogQLGlobalSettings().max_bytes_before_external_group_by is 0, so an un-overridden query does not spill, which is what kills it. This path sets 4 GiB.

Reworded to say the default settings set no spill threshold, and that the fallback stays bounded because count_settings overrides them. The same sentence was in the PR description, so I fixed it there too.